feat(agents-runtime)!: bash tool runs children with an env allowlist#4362
feat(agents-runtime)!: bash tool runs children with an env allowlist#4362msfstef wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4362 +/- ##
==========================================
+ Coverage 55.84% 55.94% +0.09%
==========================================
Files 245 245
Lines 24847 24902 +55
Branches 6878 6882 +4
==========================================
+ Hits 13876 13931 +55
Misses 10957 10957
Partials 14 14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
msfstef
force-pushed
the
security/pr4-bash-env-allowlist
branch
2 times, most recently
from
daec1c8 to
b04bc1f
Compare
Claude Code ReviewSummaryIteration 2 takes the same allowlist approach as before but absorbs nearly all of the feedback from iteration 1: the default set now covers proxy, TLS, CI/terminal, and Windows-essential vars, and the merged What's Working Well
Issues FoundCritical (Must Fix)None. Important (Should Fix)None remaining from the prior review. Suggestions (Nice to Have)A few non-secret keys still missing from the defaultsFile: The new defaults are good, but a small number of well-known non-credential keys are still stripped and have historically tripped users in this exact spot:
Not blocking; raising because the same "customer-reports-a-broken-tool, root-caused-here" failure mode applies. These can also be added in a follow-up. Commit message lists the old allowlistReference: commit The commit message body still describes the narrower v1 allowlist (PATH, HOME, USER, SHELL, LANG, LC_ALL, LC_CTYPE, TERM, TMPDIR, TMP, TEMP, XDG_*) — the proxy/TLS/Windows additions that landed in v2 aren't reflected. Cosmetic, but Changeset doesn't acknowledge what's still in scopeThe changeset accurately describes what this PR fixes (env-var leak) and the breaking-change mitigation. It doesn't mention that file-based credential exfiltration ( Negative test could lock the contract tighterFile: (Carry-over from iteration 1, still applies.) Issue ConformanceNo linked issue. PR description still references Previous Review StatusAddressed:
Not addressed (all Suggestion-level, fine to defer):
Review iteration: 2 | 2026-05-19 |
Previously `bash.ts` passed `env: { ...process.env }` wholesale to
`child_process.exec`, so any LLM-driven `echo \$ANTHROPIC_API_KEY` (or
other credential-shaped variable) would leak straight back into the
agent loop. The PR 0 characterization tests in #4354 documented this
leak; this PR replaces the wholesale passthrough with a filtered subset.
Default allowlist keeps git, npm, pnpm, and most CLI tools functional:
PATH, HOME, USER, SHELL, LANG, LC_ALL, LC_CTYPE, TERM, TMPDIR, TMP,
TEMP, XDG_CONFIG_HOME, XDG_CACHE_HOME, XDG_DATA_HOME.
New option `createBashTool(cwd, { allowedEnvKeys })` extends the default
list (it does not replace it). Customers can add `GITHUB_TOKEN` etc. on
purpose, but cannot accidentally shrink the safe defaults to a narrower
set. The extend-vs-replace decision is intentional: silently dropping
something safe like PATH would break tooling.
BREAKING: any code that relied on env-supplied credentials reaching
bash via process.env (Anthropic, OpenAI, GitHub, Brave, custom secrets)
silently stops working. This is the intended behavior — those keys
should not be reachable from LLM-controlled commands — but it requires
action. One-line mitigation:
createBashTool(cwd, { allowedEnvKeys: ['GITHUB_TOKEN'] })
Description string also stops claiming a sandbox (same change as #4355;
ensures this PR is self-contained if #4355 hasn't landed yet).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
msfstef
force-pushed
the
security/pr4-bash-env-allowlist
branch
from
b04bc1f to
a9d2786
Compare
2 participants
Summary
Closes the env-var exfiltration leak in the built-in
bashtool. Previouslybash.tspassedenv: { ...process.env }wholesale tochild_process.exec, so the LLM could exfiltrate any credential-shaped variable viaecho \$ANTHROPIC_API_KEY. Now bash spawns children with a filtered allowlist constructed once at tool creation.See
plans/sandboxing-investigation.md§1.3, §1.5, §3.5.Default allowlist
Covers shell/locale/temp/XDG basics plus categories where the absence of a variable would silently break tooling rather than improve security:
COLORTERM,NO_COLOR,FORCE_COLOR,CIHTTP_PROXY,HTTPS_PROXY,NO_PROXYplus lowercase variants (curl/git/node respect both forms)NODE_EXTRA_CA_CERTS,SSL_CERT_FILE,SSL_CERT_DIR(needed in corporate-MITM setups)SYSTEMROOT,COMSPEC,WINDIR,APPDATA,LOCALAPPDATA,USERPROFILE(cmd.exe and Node lookups fail without these)Deliberately excluded from defaults despite being commonly needed — these are real injection vectors and need an explicit opt-in:
NPM_CONFIG_*(auth tokens in npmrc-equivalent envs)GIT_SSH_COMMAND(arbitrary command injection)NODE_OPTIONS(--require ./malicious.jsis code-injection)Extension
The list extends (does not replace) the default allowlist. Customers can add keys but cannot accidentally shrink the safe defaults — silently dropping
PATHwould break tooling without a useful error.Breaking
Anything relying on env-supplied credentials reaching bash through
process.env—ANTHROPIC_API_KEY,OPENAI_API_KEY,GITHUB_TOKEN,BRAVE_SEARCH_API_KEY, custom secrets — silently stops working. Intended behavior. One-line mitigation per call viaallowedEnvKeysas above.Test plan
ANTHROPIC_API_KEYno longer appears in child outputPATHstill forwardedallowedEnvKeys: ['MY_CUSTOM']forwards that key to the childPATHstill passes when other keys are addedpnpm test,pnpm typecheck,pnpm stylecheckcleanAddressed from review
git/pnpm/network calls behind corporate proxies or on Windows by default.createBashTooltime instead of perexecute()call.process.envis still read per-call so live mutation of the parent env is honored.🤖 Generated with Claude Code